fix: apply unresolved PR2 review suggestions by Copilot · Pull Request #3 · Algodons/agentos

Copilot · 2026-05-07T16:55:26Z

Five unresolved Copilot review comments from PR #2 were never acted on before merge. This addresses all of them, plus follow-up improvements based on subsequent review feedback.

Changes

README.md

Replace corrupted •• (bad encoding) with 🤖 on line 25

Dockerfile + Kubernetes

Set USER node before WORKDIR /app in the runner stage so Docker creates /app already owned by the node user — no extra chown layer needed
Use COPY --chown=node:node for all artifacts in the runner stage; npm ci --omit=dev runs as node
Add runAsUser: 1000 / runAsGroup: 1000 to the pod-level securityContext in the K8s Deployment to satisfy runAsNonRoot: true

core/wasm/workerPool.ts

Validate maxQueueSize is a non-negative safe integer; throws on -1, NaN, Infinity, etc.

if (!Number.isSafeInteger(rawMaxQueueSize) || rawMaxQueueSize < 0) {
  throw new Error('WorkerPool: maxQueueSize must be a non-negative safe integer');
}

core/wasm/wasmRunner.ts

Validate defaultTimeoutMs and maxTimeoutMs are positive finite numbers
Enforce maxTimeoutMs >= defaultTimeoutMs at construction time

if (!Number.isFinite(defaultTimeoutMs) || defaultTimeoutMs <= 0) {
  throw new Error('WasmRunner: defaultTimeoutMs must be a positive finite number');
}
if (!Number.isFinite(maxTimeoutMs) || maxTimeoutMs <= 0) {
  throw new Error('WasmRunner: maxTimeoutMs must be a positive finite number');
}
if (maxTimeoutMs < defaultTimeoutMs) {
  throw new Error('WasmRunner: maxTimeoutMs must be >= defaultTimeoutMs');
}

tests/workerPool.load.test.ts

Add unit tests asserting invalid maxQueueSize values (-1, NaN, Infinity, fractional) throw the expected error

tests/wasm.determinism.test.ts

Add WasmRunner constructor validation suite covering non-positive/non-finite defaultTimeoutMs, non-positive/non-finite maxTimeoutMs, and maxTimeoutMs < defaultTimeoutMs

…queue validation)" Agent-Logs-Url: https://github.com/Algodons/agentos/sessions/5c427c7f-c6a3-407b-860f-916d0b08a58b Co-authored-by: SMSDAO <144380926+SMSDAO@users.noreply.github.com>

Copilot

Pull request overview

This PR addresses previously unacted Copilot review items by fixing a README encoding artifact, aligning container/Kubernetes non-root execution settings, and adding stricter runtime option validation in the WASM execution utilities.

Changes:

Fix a corrupted character in the README feature list.
Run the Docker image as the node user and align Kubernetes securityContext with UID/GID 1000.
Add input validation for WorkerPool.maxQueueSize and WasmRunner timeout configuration.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
README.md	Replaces corrupted glyph with the intended emoji in the feature list.
Dockerfile	Ensures runtime is non-root by chowning `/app` and switching to `USER node`.
deploy/k8s/base/deployment.yaml	Sets `runAsUser/runAsGroup` to match the container’s non-root user.
core/wasm/workerPool.ts	Validates `maxQueueSize` as a non-negative safe integer before assignment.
core/wasm/wasmRunner.ts	Validates timeout options and enforces `maxTimeoutMs >= defaultTimeoutMs`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+RUN chown -R node:node /app
+USER node


+    const rawMaxQueueSize = normalized.maxQueueSize ?? concurrency * 100;
+    if (!Number.isSafeInteger(rawMaxQueueSize) || rawMaxQueueSize < 0) {
+      throw new Error('WorkerPool: maxQueueSize must be a non-negative safe integer');
+    }
+    this.maxQueueSize = rawMaxQueueSize;


+    const defaultTimeoutMs = options.defaultTimeoutMs ?? 10_000;
+    const maxTimeoutMs = options.maxTimeoutMs ?? 60_000;
+
+    if (!Number.isFinite(defaultTimeoutMs) || defaultTimeoutMs <= 0) {
+      throw new Error('WasmRunner: defaultTimeoutMs must be a positive finite number');
+    }
+    if (!Number.isFinite(maxTimeoutMs) || maxTimeoutMs <= 0) {
+      throw new Error('WasmRunner: maxTimeoutMs must be a positive finite number');
+    }
+    if (maxTimeoutMs < defaultTimeoutMs) {
+      throw new Error('WasmRunner: maxTimeoutMs must be >= defaultTimeoutMs');
+    }


SMSDAO · 2026-05-07T18:05:58Z

@copilot apply changes based on the comments in this thread

…n unit tests Agent-Logs-Url: https://github.com/Algodons/agentos/sessions/43d197d7-ea70-4f58-bb0d-e8ca0714edc8 Co-authored-by: SMSDAO <144380926+SMSDAO@users.noreply.github.com>

Agent-Logs-Url: https://github.com/Algodons/agentos/sessions/43d197d7-ea70-4f58-bb0d-e8ca0714edc8 Co-authored-by: SMSDAO <144380926+SMSDAO@users.noreply.github.com>

Copilot · 2026-05-07T18:09:07Z

@copilot apply changes based on the comments in this thread

Applied all three review comments in commit 860f400 (with a follow-up cde4cfa to revert an unintentional package-lock.json change):

Dockerfile — removed RUN chown -R node:node /app. All COPY lines in the runner stage now use --chown=node:node, and USER node is set before npm ci --omit=dev so node_modules is also created with the correct ownership.
tests/workerPool.load.test.ts — added a test case asserting that -1, NaN, Infinity, and 1.5 all throw 'WorkerPool: maxQueueSize must be a non-negative safe integer'.
tests/wasm.determinism.test.ts — added a WasmRunner constructor validation suite covering: non-positive/non-finite defaultTimeoutMs, non-positive/non-finite maxTimeoutMs, and maxTimeoutMs < defaultTimeoutMs.

…tra chown layer Agent-Logs-Url: https://github.com/Algodons/agentos/sessions/66b4d71e-9b39-41cc-98ba-f3fd4b3f8018 Co-authored-by: SMSDAO <144380926+SMSDAO@users.noreply.github.com>

Copilot · 2026-05-07T18:12:02Z

@copilot apply changes based on the comments in this thread

Done in commit 7a95ba0. Here's what was applied from the three review comments:

Dockerfile — replaced RUN chown -R node:node /app with a zero-overhead approach: USER node is now set before WORKDIR /app, so Docker creates /app already owned by node. All COPY instructions use --chown=node:node and npm ci --omit=dev runs as node. No extra chown layer.
workerPool tests — added rejects invalid maxQueueSize configuration covering -1, NaN, Infinity, and fractional values.
wasmRunner tests — added WasmRunner constructor validation suite covering non-positive/non-finite defaultTimeoutMs, non-positive/non-finite maxTimeoutMs, and maxTimeoutMs < defaultTimeoutMs.

fix: apply PR2 review suggestions (emoji, k8s non-root user, timeout/…

a6e64c2

…queue validation)" Agent-Logs-Url: https://github.com/Algodons/agentos/sessions/5c427c7f-c6a3-407b-860f-916d0b08a58b Co-authored-by: SMSDAO <144380926+SMSDAO@users.noreply.github.com>

Copilot AI assigned Copilot and SMSDAO May 7, 2026

Copilot created this pull request from a session on behalf of SMSDAO May 7, 2026 16:55 View session

SMSDAO requested review from SMSDAO and Copilot May 7, 2026 16:57

Copilot started reviewing on behalf of SMSDAO May 7, 2026 16:58 View session

Copilot AI reviewed May 7, 2026

View reviewed changes

Copilot started work on behalf of SMSDAO May 7, 2026 18:06 View session

Copilot AI and others added 2 commits May 7, 2026 18:08

fix: address review comments - Dockerfile COPY --chown, add validatio…

860f400

…n unit tests Agent-Logs-Url: https://github.com/Algodons/agentos/sessions/43d197d7-ea70-4f58-bb0d-e8ca0714edc8 Co-authored-by: SMSDAO <144380926+SMSDAO@users.noreply.github.com>

fix: revert unintentional package-lock.json postcss dev flag removal

cde4cfa

Agent-Logs-Url: https://github.com/Algodons/agentos/sessions/43d197d7-ea70-4f58-bb0d-e8ca0714edc8 Co-authored-by: SMSDAO <144380926+SMSDAO@users.noreply.github.com>

Copilot finished work on behalf of SMSDAO May 7, 2026 18:09

fix: set USER node before WORKDIR so /app is owned by node without ex…

7a95ba0

…tra chown layer Agent-Logs-Url: https://github.com/Algodons/agentos/sessions/66b4d71e-9b39-41cc-98ba-f3fd4b3f8018 Co-authored-by: SMSDAO <144380926+SMSDAO@users.noreply.github.com>

Copilot finished work on behalf of SMSDAO May 7, 2026 18:12

SMSDAO marked this pull request as ready for review May 7, 2026 18:31

SMSDAO approved these changes May 7, 2026

View reviewed changes

SMSDAO merged commit 9fbd6be into main May 7, 2026
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: apply unresolved PR2 review suggestions#3

fix: apply unresolved PR2 review suggestions#3
SMSDAO merged 4 commits intomainfrom
copilot/fix-suggested-changes-from-pr2

Copilot AI commented May 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

SMSDAO commented May 7, 2026

Uh oh!

Copilot AI commented May 7, 2026

Uh oh!

Copilot AI commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

SMSDAO commented May 7, 2026

Uh oh!

Copilot AI commented May 7, 2026

Uh oh!

Copilot AI commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 7, 2026 •

edited

Loading